Add IgnoreUnusedBindingExpression quick-fix #164
Conversation
reacheight
changed the title
| { | ||
| internal partial class TryWithExpr | ||
| { | ||
| public override IType Type() => GetPsiModule().GetPredefinedType().Void; |
There was a problem hiding this comment.
let a: int =
try 1
with _ -> 1
| { | ||
| internal partial class TryFinallyExpr | ||
| { | ||
| public override IType Type() => GetPsiModule().GetPredefinedType().Void; |
There was a problem hiding this comment.
let a: int =
try 1
finally ()|
|
||
| namespace JetBrains.ReSharper.Plugins.FSharp.Psi.Impl.Tree | ||
| { | ||
| internal partial class AssertExpr |
There was a problem hiding this comment.
Please inherit from UnitExpressionBase instead.
| @@ -285,7 +301,8 @@ private void DescribeSimpleAlignmentRule((string name, CompositeNodeType nodeTyp | |||
| } | |||
|
|
|||
| private static bool IndentElseExpr(ITreeNode elseExpr, CodeFormattingContext context) => | |||
| elseExpr.GetPreviousMeaningfulSibling().IsFirstOnLine(context.CodeFormatter) && !(elseExpr is IElifExpr); | |||
| (elseExpr.GetPreviousMeaningfulSibling().IsFirstOnLine(context.CodeFormatter) | |||
| || !AreAligned(elseExpr, elseExpr.Parent?.FirstChild, context.CodeFormatter)) && !(elseExpr is IElifExpr); | |||
There was a problem hiding this comment.
Please move || to the previous line.
|
|
||
| override x.IsAvailable _ = | ||
| isValid pat && isValid letOrUseExpr && letOrUseExpr.Bindings.Count = 1 && isValid binding.Expression | ||
| && not (binding.Expression :? IDoExpr) |
There was a problem hiding this comment.
Please move && to the previous line.
| let exprToIgnore = getInnermostExpression expr | ||
| let ignoredExpr = exprToIgnore.CreateElementFactory().CreateIgnoreApp(exprToIgnore, false) | ||
| let replaced = ModificationUtil.ReplaceChild(exprToIgnore, ignoredExpr) | ||
| addParensIfNeeded replaced.LeftArgument |> ignore |
There was a problem hiding this comment.
Would it be easy to unify it with AddIgnoreFix?
| let ignoreInnermostExpression (expr: IFSharpExpression) = | ||
| let rec getInnermostExpression (expr: IFSharpExpression) = | ||
| match expr with | ||
| | :? ISequentialExpr as seqExpr -> getInnermostExpression (seqExpr.Expressions.Last()) |
There was a problem hiding this comment.
ExpressionsEnumerable is a bit preferable since it doesn't store the whole collection.
| 1{caret} | ||
| 1 |> ignore | ||
|
|
||
| () |
There was a problem hiding this comment.
Please make sure it's a correct sequence expression node after the modification, i.e. it should be something like
seqExpr
1
1 |> ignore
()
and not like
seqExpr
seqExpr
1
1 |> ignore
()
It'd be great to have this logic reused in InlineVar refactoring later.
We should probably dump the resulting tree in some of the tests with modifications.
| let {caret}c = 1 | ||
| c + c |> ignore | ||
|
|
||
| () |
There was a problem hiding this comment.
The same as above:
letExpr
binding
expr
seqExpr
c + c |> ignore
()
| override x.Text = "Inline and ignore expression" | ||
|
|
||
| override x.IsAvailable _ = | ||
| isValid pat && isValid letOrUseExpr && letOrUseExpr.Bindings.Count = 1 && isValid binding.Expression |
There was a problem hiding this comment.
It looks like it'll work on function bindings (and it shouldn't):
do
let f{caret} _ = ()
()
|
|
||
| override x.RelativeTestDataPath = "features/quickFixes/ignoreUnusedBindingExpression" | ||
|
|
||
| [<Test>] member x.``Function``() = x.DoNamedTest() |
There was a problem hiding this comment.
Let's try using NotAvailable attribute, as it was pointed in #163 (comment)?
| @@ -0,0 +1,8 @@ | |||
| module Module | |||
|
|
|||
| let a = | |||
There was a problem hiding this comment.
I think we could save up to a half of the dump by using do statement instead of the top-level binding.
module Module
do
let a =
1
1
()A bonus point would be replacing the top-level module with a anon module, but it'd require more changes in test project options, so I'd rather do it separately.
No description provided.